Skip to content

perf: optimize replace (2x faster)#23589

Merged
neilconway merged 2 commits into
apache:mainfrom
andygrove:auto-opt/replace-datafusion-20260714-182142
Jul 16, 2026
Merged

perf: optimize replace (2x faster)#23589
neilconway merged 2 commits into
apache:mainfrom
andygrove:auto-opt/replace-datafusion-20260714-182142

Conversation

@andygrove

@andygrove andygrove commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

What changes are included in this PR?

Added a scalar from/to fast path to string replace that hoists a memmem::Finder out of the per-row loop and skips materializing scalar args into N-element arrays, avoiding per-row searcher construction for the common replace(col,'lit','lit') pattern.

Are these changes tested?

Existing tests + new unit tests.

Benchmark (criterion):

  • replace_scalar from=ab [size=8192, str_len=64]: 58.573% faster (base 226815ns -> cand 93963ns)
  • replace_scalar from=the [size=8192, str_len=16]: 30.039% faster (base 155601ns -> cand 108860ns)
  • replace_scalar from=ab [size=8192, str_len=32]: 50.571% faster (base 174020ns -> cand 86017ns)
  • replace_scalar from=ab [size=8192, str_len=16]: 10.673% faster (base 135489ns -> cand 121028ns)
  • replace_scalar from=the [size=8192, str_len=32]: 52.7% faster (base 184286ns -> cand 87167ns)
  • replace_scalar from=the [size=8192, str_len=64]: 60.147% faster (base 227922ns -> cand 90834ns)

Full criterion output:

replace_scalar from="ab" [size=8192, str_len=16]
                        time:   [120.76 µs 120.96 µs 121.15 µs]
                        change: [−10.856% −10.673% −10.491%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
  12 (12.00%) low mild

replace_scalar from="the" [size=8192, str_len=16]
                        time:   [108.51 µs 108.62 µs 108.73 µs]
                        change: [−30.131% −30.039% −29.950%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) low mild

replace_scalar from="ab" [size=8192, str_len=32]
                        time:   [85.876 µs 85.955 µs 86.065 µs]
                        change: [−50.687% −50.571% −50.449%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) high mild
  3 (3.00%) high severe

replace_scalar from="the" [size=8192, str_len=32]
                        time:   [87.137 µs 87.175 µs 87.225 µs]
                        change: [−52.797% −52.700% −52.610%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low severe
  1 (1.00%) low mild
  2 (2.00%) high severe

replace_scalar from="ab" [size=8192, str_len=64]
                        time:   [93.795 µs 93.907 µs 94.016 µs]
                        change: [−58.641% −58.573% −58.502%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 18 outliers among 100 measurements (18.00%)
  4 (4.00%) low severe
  5 (5.00%) low mild
  4 (4.00%) high mild
  5 (5.00%) high severe

replace_scalar from="the" [size=8192, str_len=64]
                        time:   [90.843 µs 90.891 µs 90.957 µs]
                        change: [−60.277% −60.147% −60.021%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  2 (2.00%) low severe
  3 (3.00%) low mild
  2 (2.00%) high mild
  2 (2.00%) high severe

Are there any user-facing changes?

@github-actions github-actions Bot added the functions Changes to functions implementation label Jul 15, 2026
@andygrove andygrove changed the title perf: optimize replace in datafusion-functions perf: optimize replace (2x faster) Jul 15, 2026
@andygrove
andygrove marked this pull request as ready for review July 15, 2026 15:54
neilconway
neilconway approved these changes Jul 15, 2026
View reviewed changes

@neilconway neilconway left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement! lgtm

Comment thread datafusion/functions/src/string/replace.rs
for (from, to) in cases {
let n = rows.len();
for haystack in [
Arc::new(StringArray::from(rows.clone())) as ArrayRef,

@neilconway neilconway Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test LargeStringArray as well?

@andygrove andygrove Jul 16, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added. Thanks for catching the gap.

@alamb alamb added the performance Make DataFusion faster label Jul 15, 2026
@codecov-commenter

codecov-commenter commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.70629% with 9 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@4b68de6). Learn more about missing BASE report.

Files with missing lines Patch % Lines
datafusion/functions/src/string/replace.rs 93.70% 4 Missing and 5 partials ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #23589   +/-   ##
=======================================
  Coverage        ?   80.65%           
=======================================
  Files           ?     1086           
  Lines           ?   366524           
  Branches        ?   366524           
=======================================
  Hits            ?   295638           
  Misses          ?    53265           
  Partials        ?    17621

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@neilconway
neilconway added this pull request to the merge queue Jul 16, 2026
Merged via the queue into apache:main with commit 943e029 Jul 16, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@neilconway neilconway neilconway approved these changes

Assignees

No one assigned

Labels

functions Changes to functions implementation performance Make DataFusion faster

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andygrove @codecov-commenter @neilconway @alamb